-
Notifications
You must be signed in to change notification settings - Fork 500
Add command to add JDK to settings #4162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
rgrunber
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems mostly fine. Just be aware this overlaps with https://github.com/microsoft/vscode-java-pack/blob/main/src/java-runtime/index.ts and it probably would have been ideal to move that page here.
fbricon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please rebase against the main branch? JavaSE-25 is available now.
src/javaRuntimes.ts
Outdated
| if (config.some(r => r.path === directory[0].fsPath)) { | ||
| vscode.window.showErrorMessage(`JDK Directory ${directory[0].fsPath} already configured`); | ||
| } else { | ||
| const name = await vscode.window.showQuickPick(getSupportedJreNames(), { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would improve it by:
- detecting the java version of the selected path
- preselect the matching execution environment name
- filter out incompatible environments, i.e don't show JavaSE-22 and above if you selected a JDK 21.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also pre Java 8 support has been dropped over a year ago. Can you remove the J2SE-1.5 ... JavaSE-7 values in package.json
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
3c9b691 to
cbe1b37
Compare
src/javaRuntimes.ts
Outdated
| if (runtime) { | ||
| const config = vscode.workspace.getConfiguration('java.configuration').get('runtimes'); | ||
| if (Array.isArray(config)) { | ||
| const candidates = getSupportedJreNames().filter(name => !config.some(r => r.name === name) && compatible(runtime, name)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I would not prevent overriding existing JRE Mappings, e.g. I want to switch from one JDK 11.0.0 to 11.0.1
- versions should be sorted by most recent version 1st
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
|
Thanks @tsmaeder! |
This adds a VS Code command that
Fixes #4161